Implement Staged Tile Loading Pipeline #779
Conversation
- Change _maxSimultaneousRequests to 28 for testing - Put loadProgress calculation into ViewUpdateResult instead of being determined on the fly - Put loadProgress kick hack back in for testing
Add assertion to view results Rename some members
This was just a test. In practice, the view can change frequently. We want to drop the work that doesn't make it.
No reason to bump this. Latest tests show minimal improvement
(still much to do here)
This comment was marked as outdated.
This comment was marked as outdated.
…processLoadRequests) Previously this was only happening in dispatchMainThreadTasks, at the beginning of update_view
- Handle completed work for newly dispatched work that completes immediately (like unit tests) - Add done loading notify for tiles that fail too (but don't count towards used bytes
This comment was marked as outdated.
This comment was marked as outdated.
|
Another update to the performance test numbers... Good news: the performance wins from this PR went up! (now 25-32% improvements for the google tests) Mediocre news: although the inexplicable "warm cache is really slow" problem is gone, warm cache results are still slightly slower in this PR (9-21% slower). Denver cold cache falls into this category too. I have a pretty good reason to believe that the problem is here, which coincides with @kring 's recommendation to look out for main thread continuations. The fix would be to dispatch processing work immediately, rather than waiting to dispatch on the main thread. Not a trivial change, but not an unreasonable amount of work either. A snippet |
This comment was marked as outdated.
This comment was marked as outdated.
| // A response code of 0 is not a valid HTTP code | ||
| // and probably indicates a non-network error. |
There was a problem hiding this comment.
No, it probably indicates a valid response from a file:/// URL. It's annoying that libcurl returns a status code of 0 for a successful file read, but it does.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
…een released When unloading tiles, make sure raster mapped tiles aren't loading, if so, wait for them to finish. This case can happen when moving very quickly through the world, where a tile is unloaded before it is finished loading.
In LayerJsonTerrainLoader::getLoadWork, when no quadtree tile ID is detected, don't skip adding all work, just the url request work.
This comment was marked as outdated.
This comment was marked as outdated.
|
Any progress on this ? I can't wait for these improvements to come ! |
|
This is on hold for the time being, due to other higher-priority work and because the performance we saw with this approach was a bit mixed (some things got notably faster, but others got slower). |
|
Is there any other work being done for performance ? |
When loading tiles, implement a new staged pipeline where network requests are dispatched separately from processing work.
This achieves a 25-32% loading time reduction in some test cases.
Closes #746. Also closes #473 as a side effect.
Performance Tests Summary (Cesium for Unreal Performance Tests)
Complete Testing Data
In Depth
...
Simplified view of existing tile loads
This PR introduces this structure
The best place to start looking at the code changes would be in
Tileset:: _processWorkerThreadLoadQueue.Previous load requests were handled immediately, but now they are now passed to
TilesetContentManager, which transforms the requests into new data thatTileWorkManagercan work with, a generalizedTileLoadWork.TileWorkManagerhandles the critical logic to queue / throttle / and executeTileLoadWorkobjects.TO DO